Conversation
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| { | ||
| Logs::getInstance()->flush(); | ||
| TraceMetrics::getInstance()->flush(); | ||
| } |
There was a problem hiding this comment.
Bug: The new SentrySdk::flush() method does not flush the main transport layer, which can lead to buffered events like errors and exceptions being lost upon application exit.
Severity: CRITICAL | Confidence: High
🔍 Detailed Analysis
The new SentrySdk::flush() method is incomplete. It calls flush() on Logs and TraceMetrics, but it does not flush the main transport layer where events from captureMessage(), captureException(), and captureEvent() are buffered. Users calling this convenience method will expect all telemetry to be sent, but because the transport is not flushed, any buffered events could be lost if the application exits. This contradicts the stated goal of the method, which is to provide a single facade to flush all individual resources.
💡 Suggested Fix
Update SentrySdk::flush() to also flush the transport layer. This can be achieved by getting the current client from the hub and calling its flush() method, like so: SentrySdk::getCurrentHub()->getClient()->flush($timeout);.
🤖 Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: src/SentrySdk.php#L78-L81
Potential issue: The new `SentrySdk::flush()` method is incomplete. It calls `flush()`
on `Logs` and `TraceMetrics`, but it does not flush the main transport layer where
events from `captureMessage()`, `captureException()`, and `captureEvent()` are buffered.
Users calling this convenience method will expect all telemetry to be sent, but because
the transport is not flushed, any buffered events could be lost if the application
exits. This contradicts the stated goal of the method, which is to provide a single
facade to flush all individual resources.
Did we get this right? 👍 / 👎 to inform future reviews.
Reference ID: 7703038
stayallive
left a comment
stayallive
left a comment
There was a problem hiding this comment.
Nice DX improvement!
2 participants
Adds a
flushfacade method that will callflushon individual aggregators or buffers.If new buffers or aggregators are added, this flush method will be updated, meaning that it will no longer possible to forget to flush individual resources on the callers side if this method is used.
closes PHP-41